-
Notifications
You must be signed in to change notification settings - Fork 190
feat: add MCP server "domains" #421
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
* Rename target_domains to domains * Format files
…rs/mnovaes/add-domains
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
||
// Parse command line arguments using yargs | ||
const argv = yargs(hideBin(process.argv)) | ||
.scriptName("mcp-server-azuredevops") | ||
.usage("Usage: $0 <organization> [options]") | ||
.usage("Usage: $0 <organization> [domains...] [options]") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.usage("Usage: $0 <organization> [domains...] [options]") | |
.usage("Usage: $0 <organization> [options]") |
domains
has to be optional param, not positional
.version(packageVersion) | ||
.command("$0 <organization>", "Azure DevOps MCP Server", (yargs) => { | ||
.command("$0 <organization> [domains...] [options]", "Azure DevOps MCP Server", (yargs) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.command("$0 <organization> [domains...] [options]", "Azure DevOps MCP Server", (yargs) => { | |
.command("$0 <organization> [options]", "Azure DevOps MCP Server", (yargs) => { |
const orgUrl = "https://dev.azure.com/" + orgName; | ||
|
||
const domainsManager = new DomainsManager(argv.domains); | ||
export const enabledDomains = domainsManager.getEnabledDomains(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't find any import
for the const. Consider moving it to a local const inside getAzureDevOpsToken
"description": "Azure DevOps organization name (e.g. 'contoso')" | ||
}, | ||
{ | ||
"id": "domains", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd avoid adding an input
for new param. It creates friction for most of the users who are not planning to use it.
Instead mention it in the README, similar to --tenant
option described in https://github.com/microsoft/azure-devops-mcp/blob/main/docs/TROUBLESHOOTING.md#solution
Add domains
GitHub issue number
#386
Associated Risks
Replace by possible risks this pull request can bring you might have thought of
✅ PR Checklist
🧪 How did you test it?
Unit test added
Local run -- todo add tests images here
Context
SEP-993 is moving to Groups and Tags
We have been discussing with interested parties and authors, Groups is quite loosely coupled, which can be categorized with:
It makes sense for Groups and Tags proposal be as generic as possible. With domains we introduce a semantic group which in terms of implementation is just syntax sugar for groups, but it carries more governance on MCP servers around those.
Some open questions when Groups go to prod:
Important
More info on protocol alignment: Tool Filtering with groups and Tags
More info: SEP-1300: Tool Filtering with Groups and Tags · Issue #1300 · modelcontextprotocol/modelcontextprotocol